Skip to content

fix(common): update TS module resolution flow#1659

Open
arturovt wants to merge 7 commits into
masterfrom
fix/ts-resolution
Open

fix(common): update TS module resolution flow#1659
arturovt wants to merge 7 commits into
masterfrom
fix/ts-resolution

Conversation

@arturovt
Copy link
Copy Markdown
Collaborator

@arturovt arturovt commented Feb 4, 2024

PR Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

When loading .ts config/plugin files, ts-node is registered once per process. If two projects use different tsconfig files, the second registration is silently skipped with a warning, meaning the wrong tsconfig may be used. Additionally, registering ts-node with module: 'CommonJS' while the user's tsconfig specifies moduleResolution: 'bundler' or 'Node16' caused TS5095/TS5110 errors.

Closes #1197, #1213, #1730

What is the new behavior?

  • ts-node is registered and unregistered around each .ts file load (register → require → unregister), so each load uses the correct project tsconfig
  • moduleResolution: 'node' is added to the ts-node compiler options override to avoid TS5095/TS5110 conflicts when users specify moduleResolution: 'bundler' or 'Node16'
  • The @angular-devkit/core logger dependency is removed from common; the logger parameter is removed from loadModule()
  • Added integration test fixture: ESM package import from TypeScript webpack config (webpack-esm-plugin local package with CJS + ESM exports)

Does this PR introduce a breaking change?

[ ] Yes
[x] No

The loadModule() signature change (removing the logger parameter) is technically a breaking change for external consumers, but the logger was only used internally for the ts-node re-registration warning.

Other information

Originally authored by @arturovt. Rebased and fixed to resolve CI failures (TS5095/TS5110 moduleResolution conflict, missing webpack-esm-plugin CJS entry, removed ESM hook that caused TypeError in Angular build context).

@arturovt arturovt force-pushed the fix/ts-resolution branch 2 times, most recently from 0de7266 to b2a36d8 Compare February 4, 2024 20:55
@arturovt arturovt marked this pull request as ready for review February 4, 2024 21:05
@arturovt arturovt requested a review from just-jeb February 4, 2024 21:05
@just-jeb
Copy link
Copy Markdown
Owner

just-jeb commented Feb 5, 2024

@arturovt The CI is failing 😬.

@arturovt arturovt force-pushed the fix/ts-resolution branch 3 times, most recently from 6b5ca2f to 15c4e1b Compare March 10, 2024 21:51
@arturovt
Copy link
Copy Markdown
Collaborator Author

Ugh that CI is so weird with random failures... doesn't even show any error...

@arturovt arturovt force-pushed the fix/ts-resolution branch 5 times, most recently from f9ab45e to e02da5d Compare April 4, 2024 08:27
@arturovt
Copy link
Copy Markdown
Collaborator Author

arturovt commented Apr 4, 2024

@just-jeb could you run ci locally? Maybe you can see the actual error…

@just-jeb
Copy link
Copy Markdown
Owner

just-jeb commented Apr 4, 2024

@arturovt I'll give it a look. Were you unable to run it locally?

This commit updates the implementation for resolving `.ts` files.
Instead of registering the `ts-node` project only once, we now refrain from
doing so since there might be multiple projects with different configurations.
The current approach involves dynamically switching the implementation for
registering and unregistering the project after the `.ts` file has been transpiled
and resolved. This change addresses an issue where warnings were encountered when
`ts-node` attempted to register with different configurations. The number of configurations
is no longer a concern, as each time we need to read a `.ts` file, a new TS project is
registered. This adjustment does not impact performance or other attributes because `ts-node`
allows native project disabling. Part of the implementation has been adapted from what Nrwl Nx
already has; we can find their implementation here:
https://github.com/nrwl/nx/blob/master/packages/nx/src/plugins/js/utils/register.ts
It's worth noting that their implementation is somewhat versatile, as it also supports SWC.

Closes: #1197
Closes: #1213
Closes: #1730
@just-jeb just-jeb force-pushed the fix/ts-resolution branch from 9aaf502 to 4d1acd2 Compare May 15, 2026 12:30
just-jeb added 3 commits May 15, 2026 14:40
The root tsconfig already sets module and moduleResolution to Node16.
The redundant explicit settings in packages/common/tsconfig.json caused
compilation failures.
just-jeb added 3 commits May 15, 2026 15:39
- Add moduleResolution:'node' alongside module:'CommonJS' in ts-node options
  to fix TS5095/TS5110 when user tsconfig uses moduleResolution:'bundler' or
  'Node16' (incompatible with CommonJS in transpileOnly mode)
- Remove module.register(ts-node/esm) hook which caused TypeError in Angular
  build context
…space dep

The sanity-app-esm example uses webpack-esm-plugin to test ESM imports from
TypeScript webpack configs. The package only had an ESM export, so ts-node
(running in CJS mode) could not require() it. Added a CJS wrapper and the
require export condition, and declared the package as a file: dependency in
sanity-app-esm/package.json so Yarn resolves it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build Warning with Angular Universal - Register ts-node again

2 participants